Skip to content

fix(formatter): Honor trailing ignore comments after list separators#19925

Merged
Dunqing merged 2 commits intooxc-project:mainfrom
alubbe:trailing-comment-array-of-objects
Mar 13, 2026
Merged

fix(formatter): Honor trailing ignore comments after list separators#19925
Dunqing merged 2 commits intooxc-project:mainfrom
alubbe:trailing-comment-array-of-objects

Conversation

@alubbe
Copy link
Contributor

@alubbe alubbe commented Mar 2, 2026

Summary

After running the new oxfmt release (which comes with #19304) against our code base, we found one case where trailing ignore comments aren't properly interpreted (playground link at the end of this PR).

const items = [
  {a:aa(),b:bb(),c:cc(),d:dd(),e:ee(),f:ff(),g:gg()}, // prettier-ignore
];

Prettier keeps the code as-is, but oxfmt turns it into:

const items = [
  { a: aa(), b: bb(), c: cc(), d: dd(), e: ee(), f: ff(), g: gg() }, // prettier-ignore
];

Root cause

  • Trailing suppression detection only allowed whitespace/=/: between node end and comment.
  • In list items, there is typically a comma before the trailing comment (}, // prettier-ignore).
  • Expression formatting path also needed trailing suppression handling.

Changes

  • Extended trailing suppression gap handling to include list separators (',', ';').
  • Reused a shared internal comment scan helper to avoid duplicated logic between:
    • end-of-line comment detection
    • trailing suppression detection
  • Added trailing suppression handling for AstNode<Expression>.
  • Updated the formatter generator accordingly so regenerated code keeps this behavior.

Validation

Ran:

  • cargo test -p oxc_formatter --test mod fixtures::js::ignore
  • cargo test -p oxc_formatter --test mod fixtures::ts::ignore

Both pass.

AI disclosure

This PR was prepared with AI assistance (Codex), and all changes were reviewed and validated by the author.

https://playground.oxc.rs/?t=formatter&options=%7B%22run%22%3A%7B%22lint%22%3Atrue%2C%22formatter%22%3Atrue%2C%22transform%22%3Afalse%2C%22isolatedDeclarations%22%3Afalse%2C%22whitespace%22%3Afalse%2C%22mangle%22%3Afalse%2C%22compress%22%3Afalse%2C%22scope%22%3Atrue%2C%22symbol%22%3Atrue%2C%22cfg%22%3Afalse%7D%2C%22parser%22%3A%7B%22extension%22%3A%22tsx%22%2C%22allowReturnOutsideFunction%22%3Atrue%2C%22preserveParens%22%3Atrue%2C%22allowV8Intrinsics%22%3Atrue%2C%22semanticErrors%22%3Atrue%7D%2C%22linter%22%3A%7B%7D%2C%22formatter%22%3A%7B%22useTabs%22%3Afalse%2C%22tabWidth%22%3A2%2C%22endOfLine%22%3A%22lf%22%2C%22printWidth%22%3A80%2C%22singleQuote%22%3Afalse%2C%22jsxSingleQuote%22%3Afalse%2C%22quoteProps%22%3A%22as-needed%22%2C%22trailingComma%22%3A%22all%22%2C%22semi%22%3Atrue%2C%22arrowParens%22%3A%22always%22%2C%22bracketSpacing%22%3Atrue%2C%22bracketSameLine%22%3Afalse%2C%22objectWrap%22%3A%22preserve%22%2C%22singleAttributePerLine%22%3Afalse%7D%2C%22transformer%22%3A%7B%22target%22%3A%22es2015%22%2C%22useDefineForClassFields%22%3Atrue%2C%22experimentalDecorators%22%3Atrue%2C%22emitDecoratorMetadata%22%3Atrue%7D%2C%22isolatedDeclarations%22%3A%7B%22stripInternal%22%3Afalse%7D%2C%22codegen%22%3A%7B%22normal%22%3Atrue%2C%22jsdoc%22%3Atrue%2C%22annotation%22%3Atrue%2C%22legal%22%3Atrue%7D%2C%22compress%22%3A%7B%7D%2C%22mangle%22%3A%7B%22topLevel%22%3Atrue%2C%22keepNames%22%3Afalse%7D%2C%22controlFlow%22%3A%7B%22verbose%22%3Afalse%7D%2C%22inject%22%3A%7B%22inject%22%3A%7B%7D%7D%2C%22define%22%3A%7B%22define%22%3A%7B%7D%7D%7D&code=const+items+%3D+%5B%0A++%7Ba%3Aaa%28%29%2Cb%3Abb%28%29%2Cc%3Acc%28%29%2Cd%3Add%28%29%2Ce%3Aee%28%29%2Cf%3Aff%28%29%2Cg%3Agg%28%29%7D%2C+%2F%2F+prettier-ignore%0A%5D%3B%0A

@github-actions github-actions bot added A-ast-tools Area - AST tools A-formatter Area - Formatter C-bug Category - Bug labels Mar 2, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 2, 2026

Merging this PR will not alter performance

✅ 49 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing alubbe:trailing-comment-array-of-objects (d062500) with main (5474d0a)

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@overlookmotel overlookmotel removed their request for review March 6, 2026 00:45
@alubbe
Copy link
Contributor Author

alubbe commented Mar 10, 2026

Is there anything else that this PR needs / that you would like to see?

@Dunqing Dunqing self-assigned this Mar 10, 2026
@Dunqing
Copy link
Member

Dunqing commented Mar 10, 2026

Is there anything else that this PR needs / that you would like to see?

Sorry, I forgot to review this. I will do this before the next release (next Monday).

@Dunqing Dunqing force-pushed the trailing-comment-array-of-objects branch from 641051e to d062500 Compare March 13, 2026 07:18
@Dunqing Dunqing merged commit 4ef93ea into oxc-project:main Mar 13, 2026
28 checks passed
camc314 pushed a commit that referenced this pull request Mar 16, 2026
# Oxlint
### 🚀 Features

- c95951f linter/plugins: Implement `sourceCode.markVariableAsUsed` (#20357) (overlookmotel)
- 7a2a7d0 linter: Implement `n/handle-callback-err` rule (#19616) (Mikhail Baev)

### 🐛 Bug Fixes

- f8fbd6e linter/plugins: Remove `hashbang` property from AST (#20365) (overlookmotel)
- 6eb5b01 linter/prefer-await-to-then: Ignore Promise static methods (#20347) (camc314)
- a4b61f7 linter: Remove `defineConfig` check (#20308) (camc314)
- 3ad7f53 linter/explicit-module-boundary-types: False positive with satisfies expr (#20309) (camc314)
- f547401 linter/no-unused-private-class-members: Treat switch discriminants as read (#20307) (camc314)
- 1c07b3b diagnostics: Handle `WouldBlock` in stdout writes to prevent panic (#20295) (Boshen)

### ⚡ Performance

- e4f7248 linter: Remove unnecessary clone of owned String in drain loop (#20388) (Boshen)
- 4a67f1d linter: Eliminate Vec allocation in disable directive matching (#20387) (Boshen)
- 618a598 linter/plugins: Add fast path for files with no comments (#20366) (overlookmotel)
- b0125c5 linter/plugins: Deserialize comments without AST (#20364) (overlookmotel)
- 9cd612f linter/plugins: Recycle comment objects (#20362) (overlookmotel)
- bf442f8 linter/plugins: Cheaper `Token` creation (#20360) (overlookmotel)
- 5474d0a semantic: V8-style walk-up reference resolution (#20292) (Boshen)
- 7946eba linter/plugins: Avoid arguments spread and temp array when merging (#20318) (overlookmotel)
- fc7cf8a linter/plugins: Pre-define less CFG merger functions (#20317) (overlookmotel)
- 3b9eb28 linter/plugins: Streamline getting/creating visit fn mergers (#20319) (overlookmotel)
- f04e850 linter/plugins: Inline binary search functions into call sites (#20312) (overlookmotel)
- fe24afe linter/plugins: Apply replace globals TSDown plugin to JS files (#20305) (overlookmotel)
- 77cdacc linter/plugins: Use array buffer views for tokens (#20301) (overlookmotel)
- 910c941 linter/plugins: Reorder branches in `getTokenByRangeStart` (#20296) (overlookmotel)
- af7674c linter/tokens: Avoid extra token value allocation (#20013) (camc314)

### 📚 Documentation

- 24490b5 linter: Improve formatting for 80ish rules' docs. (#20411) (connorshea)
- 3383523 linter: Improve `--tsconfig` flag docs (#20342) (camc314)
# Oxfmt
### 🚀 Features

- d22c443 oxfmt: Export `OxfmtConfig` type (#20275) (leaysgur)
- a11ecff oxfmt/lsp: Respect `angular` language id as `.component.html` file (#20242) (Sysix)

### 🐛 Bug Fixes

- ce65099 formatter: Preserve parentheses around as expression before private field access (#20419) (bab)
- f908742 oxfmt: Revert #20326 partially (#20413) (leaysgur)
- 4ef93ea formatter: Honor trailing ignore comments after list separators (#19925) (Andreas Lubbe)
- 68fb0d0 oxfmt: Skip vite.config.ts which fails to import (#20326) (leaysgur)
- 88ee826 oxfmt: Handle literalline for script-in-vue (#20130) (leaysgur)
- 1c07b3b diagnostics: Handle `WouldBlock` in stdout writes to prevent panic (#20295) (Boshen)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast-tools Area - AST tools A-formatter Area - Formatter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants